Skip to content

quic: validate :path in sendHeaders - #64936

Open
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:quicpath
Open

quic: validate :path in sendHeaders#64936
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:quicpath

Conversation

@Archkon

@Archkon Archkon commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Fixes: #64924

@nodejs-github-bot

nodejs-github-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Aug 1, 2026
@codecov

codecov Bot commented Aug 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (a4aa3c0) to head (0f8f3bd).
⚠️ Report is 123 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##             main   #64936     +/-   ##
=========================================
  Coverage   90.28%   90.29%             
=========================================
  Files         760      759      -1     
  Lines      247061   248308   +1247     
  Branches    46592    46853    +261     
=========================================
+ Hits       223058   224201   +1143     
- Misses      15491    15522     +31     
- Partials     8512     8585     +73
Files with missing lines Coverage Δ
lib/internal/quic/quic.js 100.00% <100.00%> (ø)

... and 105 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Ethan-Arrowood
Ethan-Arrowood reviewed Aug 6, 2026
View reviewed changes
Comment thread lib/internal/quic/quic.js
@Archkon
quic: validate :path in sendHeaders
0f8f3bd 
Reject invalid :path values before passing headers to nghttp3.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
@Archkon
Archkon requested a review from Ethan-Arrowood August 7, 2026 07:49
jasnell
jasnell reviewed Aug 7, 2026
View reviewed changes
Comment thread lib/internal/quic/quic.js
@Archkon

Archkon commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

node/src/quic/http3.cc

Lines 543 to 597 in a4aa3c0

bool SendHeaders(const Stream& stream,
HeadersKind kind,
const Local<Array>& headers,
HeadersFlags flags = HeadersFlags::NONE) override {
Session::SendPendingDataScope send_scope(&session());
Http3Headers nva(env(), headers);
switch (kind) {
case HeadersKind::HINTS: {
if (!session().is_server()) {
// Client side cannot send hints
return false;
}
Debug(&session(),
"Submitting %" PRIu64 " early hints for stream %" PRIu64,
stream.id());
return nghttp3_conn_submit_info(
*this, stream.id(), nva.data(), nva.length()) == 0;
break;
}
case HeadersKind::INITIAL: {
static constexpr nghttp3_data_reader reader = {on_read_data_callback};
const nghttp3_data_reader* reader_ptr = nullptr;
// If the terminal flag is set, that means that we know we're only
// sending headers and no body and the stream writable side should be
// closed immediately because there is no nghttp3_data_reader provided.
if (flags != HeadersFlags::TERMINAL) {
reader_ptr = &reader;
}
if (session().is_server()) {
// If this is a server, we're submitting a response...
Debug(&session(),
"Submitting %" PRIu64 " response headers for stream %" PRIu64,
nva.length(),
stream.id());
return nghttp3_conn_submit_response(*this,
stream.id(),
nva.data(),
nva.length(),
reader_ptr) == 0;
} else {
// Otherwise we're submitting a request...
Debug(&session(),
"Submitting %" PRIu64 " request headers for stream %" PRIu64,
nva.length(),
stream.id());
return nghttp3_conn_submit_request(*this,
stream.id(),
nva.data(),
nva.length(),
reader_ptr,
const_cast<Stream*>(&stream)) == 0;
}

@jasnell Would this be better place to validate the :path field ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell left review comments
@Ethan-Arrowood Ethan-Arrowood Awaiting requested review from Ethan-Arrowood

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

quic: sendHeaders with non conforming path

4 participants

@Archkon @nodejs-github-bot @jasnell @Ethan-Arrowood